-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[Draft] Summary Based Analysis Prototype #144224
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
… the summaries from
…ry consumer logic
|
|
You can test this locally with the following command:git-clang-format --diff HEAD~1 HEAD --extensions cpp,h -- clang/include/clang/Summary/SummaryAttribute.h clang/include/clang/Summary/SummaryConsumer.h clang/include/clang/Summary/SummaryContext.h clang/include/clang/Summary/SummarySerialization.h clang/lib/Summary/SummaryAttribute.cpp clang/lib/Summary/SummaryConsumer.cpp clang/lib/Summary/SummaryContext.cpp clang/lib/Summary/SummarySerialization.cpp clang/include/clang/Frontend/CompilerInstance.h clang/include/clang/Frontend/FrontendOptions.h clang/include/clang/Sema/Sema.h clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h clang/lib/Driver/ToolChains/Clang.cpp clang/lib/Frontend/CompilerInstance.cpp clang/lib/Frontend/CompilerInvocation.cpp clang/lib/Frontend/FrontendAction.cpp clang/lib/Sema/Sema.cpp clang/lib/Sema/SemaDecl.cpp clang/lib/StaticAnalyzer/Core/CallEvent.cpp clang/lib/StaticAnalyzer/Core/ExprEngine.cpp clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cppView the diff from clang-format here.diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index 124ffc755..24907eec1 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -5320,11 +5320,11 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
StringRef ObjDir = llvm::sys::path::parent_path(FinalOutput->getValue());
if (!ObjDir.empty())
EmitSummaryDir = ObjDir;
- }
-
- if(A->containsValue("src") && Input.isFilename()) {
+ }
+
+ if (A->containsValue("src") && Input.isFilename()) {
StringRef BasePath = llvm::sys::path::parent_path(Input.getFilename());
- if(!BasePath.empty())
+ if (!BasePath.empty())
EmitSummaryDir = BasePath.str();
}
diff --git a/clang/lib/Frontend/CompilerInstance.cpp b/clang/lib/Frontend/CompilerInstance.cpp
index b4124d202..926705416 100644
--- a/clang/lib/Frontend/CompilerInstance.cpp
+++ b/clang/lib/Frontend/CompilerInstance.cpp
@@ -748,8 +748,9 @@ void CompilerInstance::createSummaryConsumer(FrontendInputFile Input) {
return;
llvm::SmallString<32> SummaryFile = EmitSummaryDir;
- llvm::sys::path::append(SummaryFile, llvm::sys::path::filename(Input.getFile()));
-
+ llvm::sys::path::append(SummaryFile,
+ llvm::sys::path::filename(Input.getFile()));
+
StringRef Format = getFrontendOpts().SummaryFormat;
llvm::sys::path::replace_extension(SummaryFile,
Format == "binary" ? "summary" : Format);
|
Xazax-hun
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did not have time to look at it properly, just some small drive by comments.
| return *SummaryCtx; | ||
| } | ||
|
|
||
| void createSummaryContext() { SummaryCtx.reset(new SummaryContext()); } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we assert here that we don't have a summary context yet?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure tbh. I just mirrored what other create functions such as CompilerInstance::createSema() do. Those don't assert the existence, so maybe the consumers of the class expect this kind of behaviour.
|
|
||
| namespace clang { | ||
| class FunctionSummary { | ||
| SmallVector<char> ID; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why SmallVector instead of a string type?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is just an artifact from index::generateUSRForDecl() expecting a SmallVectorImpl<char> & as it's buffer. In the very beginning, the constructor of this class took a FunctionDecl and generated the USR into this field.
This and every other data structure used needs to be revisited throught these source files once again before the patch is finalized.
| FunctionSummary(SmallVector<char> ID, std::set<const SummaryAttr *> Attrs, | ||
| std::set<SmallVector<char>> Calls); | ||
|
|
||
| SmallVector<char> getID() const { return ID; } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you want to return a StringRef/ArrayRef instead?
| }); | ||
| }); | ||
| } | ||
| } // namespace clang No newline at end of file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: missing new line.
|
FYI I really wish to come back to this PR, but I'm really busy. |
afa0ada to
bc4b57d
Compare
This is the initial implementation of how I imagine summaries used in clang and the CSA. There are still rooms for improvement and the patch still serves as an RFC to discuss which direction to go further.
1.) Compile each TU to generate the summaries
2.) Give the path to the directory containing the summaries to a clang tool to use them
The summaries for
main.cppandfoo.cpplook like this:[ { "id": "c:@F@main#", "attrs": { "function": [] }, "calls": [ { "id": "c:@F@foo#" } ] } ][ { "id": "c:@F@foo#", "attrs": { "function": [ "no_write_global" ] }, "calls": [] } ]